Claude finished @jonathangreen's task in 3m 29s —— View job

Code Review

• Gather context (read PR diff and changed files)
• Check for dangling references to v5/v6/v7
• Verify test migration fidelity
• Review revision directory / mock changes
• Post review

Summary

Clean, mechanical removal. I verified there are no remaining Python references to SearchV5/SearchV6/SearchV7 anywhere in src/ or tests/, that SearchV8 is genuinely self-contained (no imports from the deleted modules, AUTHOR_CHAR_FILTER_NAMES/CHAR_FILTERS are defined on SearchV8 itself so the migrated test binds correctly), and that the only runtime consumers of the directory — celery/tasks/search.py:120 and scripts/initialization.py:140 — call highest(), so trimming REVISIONS to a single entry doesn't change any lookup path. The char-filter test is a faithful port (identical assertions, just repointed at SearchV8), and import re moved with it. The release-ordering guard in the description is the right call per CLAUDE.md's online-migration rule. Two minor items below; neither blocks.

Details

Minor: docker/Dockerfile.os2.ci:5

Both CI OpenSearch images justify installing analysis-icu with a pointer to src/palace/manager/search/v5.py, which this PR deletes — the reference is now dangling. v8.py is what needs the plugin (icu_collation_keyword at v8.py:224 and the icu_collation filter at v8.py:198), so repointing both comments at v8.py keeps them useful:

# analysis-icu is required by the search schema (see src/palace/manager/search/v8.py).

Minor: tests/manager/search/test_search_v8.py:29-39 (removed)

Dropping test_mapping_matches_v7 is right (there's nothing left to compare against), but it was the only test pinning v8's mapping structure — what remains asserts the version, the base class, the index settings keys, and the char filters, none of which cover the field set or analysis config. Since revisions are documented as immutable once created (revision.py:5-11), a stray edit to v8.py after release would silently diverge from production indexes without a version bump, and only the docker end-to-end suite would have any chance of catching it. A cheap replacement guard is a snapshot assertion over the serialized document:

def test_mapping_is_stable(self):
    """v8 is released; its mapping must not change without a new revision."""
    document = SearchV8().mapping_document()
    assert set(document.properties) == {"summary", "title", ...}
    assert document.serialize_properties() == EXPECTED_V8_PROPERTIES

Note: I couldn't execute the test suite in this environment (sandbox blocked pytest), so my review is static analysis of the diff plus reading the affected call sites; CI and Codecov on this PR are green.
| Branch: chore/remove-old-search-schema-revisions

Greptile Summary

This PR removes the now-superseded v5, v6, and v7 search schema revision modules, their tests, and all references to them, leaving REVISIONS = [SearchV8()] as the sole entry in SearchRevisionDirectory. It is the companion cleanup to #3522, which made v8 self-contained.

• Deletes v5.py, v6.py, v7.py and their dedicated test files; removes the old entries from REVISIONS.
• Ports the author char-filter test from the deleted TestSearchV5 into TestSearchV8, and drops the now-unnecessary v7-equivalence test.
• Updates MockSearchSchemaRevisionLatest and the name_for_index docstring to reference v8.

Confidence Score: 5/5

Safe to merge — this is a straightforward dead-code removal with no functional changes to production paths.

All v5/v6/v7 references have been scrubbed from the codebase (confirmed via grep), v8 is self-contained and unchanged functionally, tests were ported correctly, and the mock backing document now matches the current production schema.

Files Needing Attention: No files require special attention.

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    RD["SearchRevisionDirectory\n(revision_directory.py)"]
    V8["SearchV8\n(v8.py)"]
    SR["SearchSchemaRevision\n(revision.py)"]

    RD -->|"REVISIONS = [SearchV8()]"| V8
    V8 -->|inherits| SR

    subgraph deleted["Deleted (this PR)"]
        V5["SearchV5 (v5.py)"]
        V6["SearchV6 (v6.py)"]
        V7["SearchV7 (v7.py)"]
        V6 -->|"was: inherits"| V5
        V7 -->|"was: inherits"| V6
    end

    subgraph mocks["Test Mocks"]
        MSSL["MockSearchSchemaRevisionLatest"]
        MSSL -->|"_document = SearchV8().mapping_document()"| V8
    end

    style deleted fill:#ffdddd,stroke:#cc0000,color:#333

_{Reviews (3): Last reviewed commit: "Trim stale revision-chain history from t..." | Re-trigger Greptile}

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.50%. Comparing base (53e3a1d) to head (2b54d93).
⚠️ Report is 31 commits behind head on main.

@@            Coverage Diff             @@
##             main    #3523      +/-   ##
==========================================
- Coverage   93.52%   93.50%   -0.02%     
==========================================
  Files         512      509       -3     
  Lines       46760    46663      -97     
  Branches     6379     6378       -1     
==========================================
- Hits        43731    43633      -98     
- Misses       1958     1959       +1     
  Partials     1071     1071              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

I'm going to leave this in draft until its safe to merge, but its ready for a code review now, then it can just wait to get merged once approved.